Skip to content

Update the qobj schema to support pulse gate calibrations.#4761

Merged
mergify[bot] merged 43 commits into
Qiskit:masterfrom
lcapelluto:pulse-gates/schema
Aug 21, 2020
Merged

Update the qobj schema to support pulse gate calibrations.#4761
mergify[bot] merged 43 commits into
Qiskit:masterfrom
lcapelluto:pulse-gates/schema

Conversation

@lcapelluto
Copy link
Copy Markdown
Contributor

@lcapelluto lcapelluto commented Jul 20, 2020

Summary

Begin Pulse Gates feature by updating the schema. Calibrations may be in the job config or the experiment config.

Example:

// job: QasmQobj
// job.config
{
  'pulse_library': [
    // same as pulse lib for PulseQobj
  ],
  'calibrations': {'gates': [
      // see below
    ],
  }
}

// job.experiments[0].config.calibrations
{
  'gates: [
    {"name": "rxtheta", "qubits": [0], "params": [3.14], "instructions": [openpulse_instructions]},
    {"name": "rxtheta", "qubits": [0], "params": [1.57], "instructions": [openpulse_instructions]},
    {"name": "rxtheta", "qubits": [1], "params": [1.57], "instructions": [openpulse_instructions]}
  ]
}

The 'gates' information must be sufficient to match against any QASM Qobj instruction within the job.experiments[i].instructions fields, for instance: QasmQobjInstruction(name='rxtheta', params=[1.57], qubits=[1]) matches the last entry.

We can move pulse_library to the top level to avoid duplicating memory-intensive pulse waveforms in a single job. The assembler will assert that each waveform in the pulse library is referenced uniquely.

Details and comments

  • TODO: add an example json
  • TODO: GateCalibration object
  • TODO: Move pulse_library to qobj level
  • TODO: add qobj.config.calibrations field
  • Update example json with some top level calibrations

Limitations

The current iteration supports any possible job, however, it is of course less efficient than a parameterized Qobj would be.

This schema cannot support unbound parameters, which is also true for any circuit at the moment, except here: Qiskit/qiskit-aer#485. Supporting unbound parameters should be done in a follow up (e.g., a gate's params are not fully specified and a value in the openpulse_instructions depends on that param). This follow up should be useable/compatible with Pulse Schedule, as well.

@lcapelluto lcapelluto marked this pull request as ready for review July 20, 2020 20:43
@lcapelluto lcapelluto requested a review from a team as a code owner July 20, 2020 20:43
Comment thread qiskit/qobj/qasm_qobj.py Outdated
Comment thread qiskit/schemas/qobj_schema.json
@taalexander taalexander added the type: enhancement It's working, but needs polishing label Jul 20, 2020
@taalexander taalexander self-assigned this Jul 20, 2020
Comment thread qiskit/qobj/qasm_qobj.py Outdated
Comment thread qiskit/qobj/qasm_qobj.py Outdated
Comment thread qiskit/schemas/qobj_schema.json
Comment thread qiskit/schemas/qobj_schema.json
@lcapelluto lcapelluto mentioned this pull request Aug 10, 2020
2 tasks
Comment thread qiskit/qobj/__init__.py
Comment thread qiskit/qobj/qasm_qobj.py Outdated
Copy link
Copy Markdown
Contributor

@taalexander taalexander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good except for needing top-level calibrations and my one comment regarding the lazy loading

Comment thread qiskit/qobj/common.py
Copy link
Copy Markdown
Member

@SooluThomas SooluThomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicks

Comment thread qiskit/qobj/common.py Outdated
Comment thread qiskit/qobj/qasm_qobj.py Outdated
Co-authored-by: SooluThomas <soolu.elto@gmail.com>
@lcapelluto lcapelluto requested a review from SooluThomas August 18, 2020 16:11
Copy link
Copy Markdown
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM except for one backwards compatibility concern around moving the validator module property. I know that's being used downstream by people and we shouldn't break them unless we have to. We either should alias and deprecate it with a release note or just leave it in qobj/qasm.py.

Comment thread qiskit/qobj/common.py
Comment thread qiskit/qobj/qasm_qobj.py
@lcapelluto lcapelluto requested a review from mtreinish August 19, 2020 13:56
Copy link
Copy Markdown
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for checking the deepcopy/pickling

Copy link
Copy Markdown
Contributor

@taalexander taalexander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, all suggestions are non-blocking.

Comment thread qiskit/qobj/qasm_qobj.py
self.schema_version = '1.3.0'

def _validate_json_schema(self, out_dict):
class QobjEncoder(json.JSONEncoder):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be broken out as a separate utility class?

],
"instructions": [
{
"name": "1c5a0346ff72c04c45d536d50d13b0d42cf65dc8e320d32385d36861d3bb1e41",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These hashed names are pretty cryptic for an example.

Comment thread qiskit/qobj/qasm_qobj.py
Comment on lines +412 to +413
"""
Initialize a single gate calibration. Instructions may reference waveforms which should be
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""
Initialize a single gate calibration. Instructions may reference waveforms which should be
"""Initialize a single gate calibration. Instructions may reference waveforms which should be

"type": "object",
"properties": {
"gates": {
"description": "A list of dicts which contain all the required information to lower a gate to waveforms.",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"description": "A list of dicts which contain all the required information to lower a gate to waveforms.",
"description": "A list of objects which contain all the required information to lower a gate to waveforms.",

"type": "object",
"properties": {
"gates": {
"description": "A list of dicts which contain all the required information to lower a gate to waveforms.",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"description": "A list of dicts which contain all the required information to lower a gate to waveforms.",
"description": "A list of objects which contain all the required information to lower a gate to waveforms.",

@mergify mergify Bot merged commit 68f65d2 into Qiskit:master Aug 21, 2020
@1ucian0 1ucian0 added the mod: pulse Related to the Pulse module label Jan 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mod: pulse Related to the Pulse module type: enhancement It's working, but needs polishing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants